Skip to content

fix(backup): contain restore file paths within the handler root#9760

Open
alhudz wants to merge 1 commit into
dgraph-io:mainfrom
alhudz:backup-handler-path-containment
Open

fix(backup): contain restore file paths within the handler root#9760
alhudz wants to merge 1 commit into
dgraph-io:mainfrom
alhudz:backup-handler-path-containment

Conversation

@alhudz

@alhudz alhudz commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

Description

The path field of a backup manifest is read back from the backup location (readMasterManifest) and used as-is during restore.

  1. restore_map.go joins manifest.Path with the backup file name and streams it through fileHandler.Stream -> os.Open(filepath.Join(rootDir, prefix, path)).
  2. getManifestsToRestore joins the same manifest.Path for FileExists, and the s3/minio handler builds object keys the same way in getObjectPath.

JoinPath/getObjectPath do not contain the relative path, so a manifest left in the backup target with path set to ../../.. steers those reads outside the handler root. The fix sits in the handlers because that is the single choke point every backup/restore file op goes through.

Repro: worker/backup_handler_test.go (fails before, passes after).
Expected: the resolved path stays under the handler root.
Actual: JoinPath("../../../../etc/passwd") returns /etc/passwd; getObjectPath returns ../../etc/passwd.
Fix: cleanRelPath anchors the path at the root and drops a leading separator / .. before the join; JoinPath (file + s3) and getObjectPath all route through it.

Checklist

  • The PR title follows the
    Conventional Commits syntax, leading
    with fix:, feat:, chore:, ci:, etc.
  • Code compiles correctly and linting (via trunk) passes locally
  • Tests added for new functionality, or regression tests for bug fixes added as applicable

@alhudz alhudz requested a review from a team as a code owner June 20, 2026 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant